to_array_object: type dtype as str, note possibilities #213
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcoGorelli
force-pushed
the
specify-array-api-dtypes
branch
from
b01dec1 to
578ac2f
Compare
rgommers
reviewed
Aug 2, 2023
|
sure sounds good |
rgommers
reviewed
Aug 2, 2023
| The dtype of the array-API-compliant object to return. | ||
| Must be one of: | ||
|
|
||
| - namespace.Bool() |
There was a problem hiding this comment.
the namespace. seems confusing to me, and I don't think we've used that anywhere else. One of these should work:
Must be an instance of
- `Bool`
- ` Int8`
- etc
or, more concisely:
Must be a boolean or numerical (signed or unsigned integer, or floating-point) dtype that is part of this standard.
There was a problem hiding this comment.
thanks - sure I've removed namespace
do we want to keep Bool() to remind that it's a class they need to pass?
There was a problem hiding this comment.
sure, sounds good to me. I think it works like it is now.
rgommers
approved these changes
Aug 3, 2023
|
thanks for your review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
not all dataframe dtypes are supported by the array api (e.g.
datetime), so just accepting the same ones and sometimes raisingNotImplementedErrormay not be the best experienceHow about just accepting
str, and listing the allowed strings? These dtypes don't need to store anything (like categories / time unit / time zone), so don't need to be classes anyway.Internally, each implementation can map to the correct object (e.g. for pandas: 'int64' would go to np.int64)